-
Notifications
You must be signed in to change notification settings - Fork 47
chore: Bump Perception to 2.0 #376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
.package(url: "https://github.com/pointfreeco/swift-identified-collections", from: "1.0.0"), | ||
.package(url: "https://github.com/pointfreeco/swift-macro-testing", from: "0.4.0"), | ||
.package(url: "https://github.com/pointfreeco/swift-perception", from: "1.5.0"), | ||
.package(url: "https://github.com/pointfreeco/swift-perception", from: "2.0.0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(commenting here for threading support) – just an FYI, but i believe the 'memory leak fix' was reverted from the 6.2 release branch (see here). idk exactly how that corresponds to what's in any given Xcode beta. sadly no justification was given in any of the PRs so not sure what the underlying motivation was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting. It looks like the pointfree folks haven't removed it from their backport at this point...
Pinged them about it here: pointfreeco/swift-perception#153
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll look into getting those changes into Perception soon. I pinged their PR for more context, so hopefully they can provide some.
if tracker.isComplete == false { | ||
XCTFail("state.child should not change.") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same kind of approach used in Perception now: https://github.com/pointfreeco/swift-perception/blob/main/Tests/PerceptionTests/PerceptionTrackingTests.swift#L32
The issue is that the registrar deinit pathway will now trigger an onChange which we don't really want to "count" in this context.
Related to pointfreeco/swift-perception#153

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the tests on which this file is based appear to not have been updated in this manner – any idea why this wasn't necessary there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests were written in a way that passed with the old behavior, too. When I asked the Swift team about the revert they said they are looking to restore some of the behavior soon, so I'm just minimizing churn while waiting for things to land.
Closing in favor of #377 |
## Checklist **Bump perception to 2.0.6** **`tuist install --path Samples` to update Sample resolved dependencies** The Perception version specified in Package.swift is now `1.5.0"..<"3.0.0`. This allows easier upgrade and downgrades by consumers. I've opened this in favor of #376 because Perception has been [updated](pointfreeco/swift-perception#156) following Apple's reversion of swiftlang/swift#83436 and it avoids test updates on our end for now. I didn't run the `tuist install` with the `--update` flag this time either. There doesn't seem to be a way to scope the updates to only Perception and it's sub-dependencies and I didn't want all the other clutter associated with this change. - [x] Unit Tests - [x] UI Tests - [x] Snapshot Tests (iOS only) - [x] I have made corresponding changes to the documentation - [x] Resolve new Perception warning in ios-register: squareup/ios-register#131811 - [x] Resolve new instances of Perception warnings in Market: squareup/market#11042 --------- Co-authored-by: Andrew Watt <100192+watt@users.noreply.github.com>
tuist install --update --path Samples
to update Sample resolved dependenciesThis bumps the pointfreeco/swift-perception dependency to 2.0 which includes a memory leak fix and improved perception check performance (that our developers have complained about in the past).
Checklist